Skip to content

Conversation

flip1995
Copy link
Member

Found that this enum is not Copy while reviewing this Clippy PR: rust-lang/rust-clippy#5727 (comment)

There shouldn't be a reason why this should not be Copy.

@rust-highfive
Copy link
Contributor

r? @ecstatic-morse

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 22, 2020
@Manishearth
Copy link
Member

@bors r+ rollup=always

@bors
Copy link
Collaborator

bors commented Jun 26, 2020

📌 Commit 2792014 has been approved by Manishearth

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 26, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 26, 2020
…arth

Rollup of 13 pull requests

Successful merges:

 - rust-lang#72620 (Omit DW_AT_linkage_name when it is the same as DW_AT_name)
 - rust-lang#72967 (Don't move cursor in search box when using arrows to navigate results)
 - rust-lang#73102 (proc_macro: Stop flattening groups with dummy spans)
 - rust-lang#73297 (Support configurable deny-warnings for all in-tree crates.)
 - rust-lang#73507 (Cleanup MinGW LLVM linkage workaround)
 - rust-lang#73588 (Fix handling of reserved registers for ARM inline asm)
 - rust-lang#73597 (Record span of `const` kw in GenericParamKind)
 - rust-lang#73629 (Make AssocOp Copy)
 - rust-lang#73681 (Update Chalk to 0.14)
 - rust-lang#73707 (Fix links in `SliceIndex` documentation)
 - rust-lang#73719 (emitter: column width defaults to 140)
 - rust-lang#73729 (disable collectionbenches for android)
 - rust-lang#73748 (Add code block to code in documentation of `List::rebase_onto`)

Failed merges:

r? @ghost
@bors bors merged commit 3d7521d into rust-lang:master Jun 26, 2020
@flip1995 flip1995 deleted the assoc_op_copy_clone branch June 26, 2020 15:12
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 14, 2020
…arth

Rollup of 13 pull requests

Successful merges:

 - rust-lang#72620 (Omit DW_AT_linkage_name when it is the same as DW_AT_name)
 - rust-lang#72967 (Don't move cursor in search box when using arrows to navigate results)
 - rust-lang#73102 (proc_macro: Stop flattening groups with dummy spans)
 - rust-lang#73297 (Support configurable deny-warnings for all in-tree crates.)
 - rust-lang#73507 (Cleanup MinGW LLVM linkage workaround)
 - rust-lang#73588 (Fix handling of reserved registers for ARM inline asm)
 - rust-lang#73597 (Record span of `const` kw in GenericParamKind)
 - rust-lang#73629 (Make AssocOp Copy)
 - rust-lang#73681 (Update Chalk to 0.14)
 - rust-lang#73707 (Fix links in `SliceIndex` documentation)
 - rust-lang#73719 (emitter: column width defaults to 140)
 - rust-lang#73729 (disable collectionbenches for android)
 - rust-lang#73748 (Add code block to code in documentation of `List::rebase_onto`)

Failed merges:

r? @ghost
bors added a commit to rust-lang/rust-clippy that referenced this pull request Oct 10, 2020
Expands `manual_memcpy` to lint ones with loop counters

Closes #1670

This PR expands `manual_memcpy` to lint ones with loop counters as described in #1670 (comment)

Although the current code is working, I have a couple of questions and concerns.

~~Firstly, I manually implemented `Clone` for `Sugg` because `AssocOp` lacks `Clone`. As `AssocOp` only holds an enum, which is `Copy`, as a value, it seems `AssocOp` can be `Clone`; but, I was not sure where to ask it. Should I make a PR to `rustc`?~~ The [PR]( rust-lang/rust#73629) was made.

Secondly, manual copying with loop counters are likely to trigger `needless_range_loop` and `explicit_counter_loop` along with `manual_memcpy`; in fact, I explicitly allowed them in the tests. Is there any way to disable these two lints when a code triggers `manual_memcpy`?

And, another thing I'd like to note is that `Sugg` adds unnecessary parentheses when expressions with parentheses passed to its `hir` function, as seen here:

```
error: it looks like you're manually copying between slices
  --> $DIR/manual_memcpy.rs:145:14
   |
LL |     for i in 3..(3 + src.len()) {
   |              ^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)])
```

However, using the `hir` function is needed to prevent the suggestion causing  errors when users use bitwise operations; and also this have already existed, for example: `verbose_bit_mask`. Thus, I think this is fine.

changelog: Expands `manual_memcpy` to lint ones with loop counters
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants